Conversation
jakecoffman
approved these changes
May 11, 2022
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Assuming a project has the following config:
dependabot.yml
In their package.json they have a dependency from their private registry, which has a
resolvedURL in the form:This will result in confusing behaviour where we will correctly find the latest version in the private registry but fail to actually update the package with a
private_source_authentication_failure.The problem
The
private_source_authentication_failureis actually a misdirection, using the debugger I was able to output the raw error which was coming from npm hereThis indicates that we are actually calling the public npm registry and getting a 404 but then assume that it's a private registry problem here as this line is false:
The fix
After investigating I discovered that no 'scoped registry' line was being added to the
.npmrcfile we generate on the fly when performing the update.It should have had a line like so:
The reason this wasn't being added is that due to a fix for a duplicate trailing slash in the past (https://github.com/dependabot/dependabot-core/pull/1013/files), we started adding a trailing slash to the
registryvalue used in this method unintentionally:dependabot-core/npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npmrc_builder.rb
Lines 174 to 198 in 3428ef7
The fix was to add the compulsory trailing slash later in the method, I've added test cases that verify this resolves the issue.
Unrelated changes
I fixed the bug in fd4fb97 to make the red/green pass as clear as possible. The final commit cleans up some test churn where we expected the scoped registry lines to have the trailing
/- this isn't necessary and we tolerate it as a side-effect of the original bug fix.